[SPARK-56744][INFRA] Document test base class hierarchy in AGENTS.md#55707
Open
zhengruifeng wants to merge 11 commits intoapache:masterfrom
Open
[SPARK-56744][INFRA] Document test base class hierarchy in AGENTS.md#55707zhengruifeng wants to merge 11 commits intoapache:masterfrom
zhengruifeng wants to merge 11 commits intoapache:masterfrom
Conversation
Add a "Test Base Classes" section to AGENTS.md describing the layered SparkFunSuite -> PlanTest -> QueryTest -> SharedSparkSession hierarchy, when to pick each base, common redundant-mixin patterns to avoid, and the linearization gotcha when the first item in an `extends` clause is a pure trait without the SparkFunSuite class chain. Generated-by: Claude opus-4-7
…sion) in AGENTS.md Clarify that QueryTest declares `spark` abstractly and that `SharedSparkSession` is one of several session-providing traits in the repo. List the common providers (`SharedSparkSession`, `TestHiveSingleton`, `RemoteSparkSession`) and note the rare manual-override pattern. Generated-by: Claude opus-4-7
…ENTS.md Add a "Base chain" column to the session provider table and explain that RemoteSparkSession directly extends AnyFunSuite (not SparkFunSuite), so Connect client suites must combine it with QueryTest to recover the SparkFunSuite-only features (per-test timeout, gridTest, retry, log-capture). Generated-by: Claude opus-4-7
…S.md Add SparkTestSuite as the bottom of the test base class chain — it holds the common Spark test functionality (thread audit, fixed timezone/locale, withTempDir, withLogAppender, checkError) and is style-agnostic. SparkFunSuite is AnyFunSuite + SparkTestSuite. Mention the alternative-style demonstration suites under core/.../*SparkTestSuite.scala and update the linearization gotcha to note that SparkTestSuite is itself a pure trait. Generated-by: Claude opus-4-7
…TS.md Drop the RemoteSparkSession row and follow-up paragraph from the session provider section. The trait does not formally extend SparkSessionProvider and serves a separate audience (Connect client tests under sql/connect/client/jvm), so it is out of scope for this guide. Generated-by: Claude opus-4-7
QueryTest extends SparkFunSuite with QueryTestBase with PlanTest, so PlanTest is a parent of QueryTest. Indent QueryTest one level deeper to reflect the actual hierarchy and show QueryTest as PlanTest + QueryTestBase (PlanTest already brings the SparkFunSuite chain). Generated-by: Claude opus-4-7
Add a parallel diagram for the style-agnostic `*Base` helper traits (`PlanTestBase` -> `QueryTestBase` -> `SharedSparkSessionBase`) and note that `PlanTest` / `QueryTest` / `SharedSparkSession` are the FunSuite-flavored bundles built on top of them, analogous to `SparkFunSuite` = `AnyFunSuite` + `SparkTestSuite`. Generated-by: Claude opus-4-7
Drop the "manual session override", helper-trait combination, and "common mistakes" subsections from the Test Base Classes section to keep the guide focused on the layered base hierarchy and the linearization gotcha. Generated-by: Claude opus-4-7
Reorder the Test Base Classes section so the style-agnostic helper traits (SparkTestSuite, PlanTestBase, QueryTestBase, SharedSparkSessionBase) are introduced first, followed by the class-bearing FunSuite-style bases that bundle each helper with AnyFunSuite. The helpers are where the actual test functionality lives, so leading with them makes the relationship clearer. Generated-by: Claude opus-4-7
The section covers Scala test bases only (PySpark tests are handled separately under Build and Test), so the more specific name avoids confusion. Generated-by: Claude opus-4-7
HyukjinKwon
approved these changes
May 6, 2026
cloud-fan
reviewed
May 7, 2026
| SparkTestSuite (core; thread audit, fixed timezone/locale, withTempDir, withLogAppender, checkError) | ||
| PlanTestBase (sql/catalyst; plan-comparison helpers — comparePlans, normalizePlan) | ||
| <- QueryTestBase (sql/core; adds SQL/DataFrame helpers + abstract `spark` via `SparkSessionProvider`) | ||
| <- SharedSparkSessionBase (sql/core; provides the actual `TestSparkSession`) |
Contributor
There was a problem hiding this comment.
hmmm do we need to mention then? Inside Spark we should always pick AnyFunSuite style for consistency
Contributor
Author
There was a problem hiding this comment.
Good point — agreed. Dropped the style-agnostic *Base chain (and the related SparkTestSuite row + linearization-gotcha example) in 65bb5e6. The section now only documents the SparkFunSuite -> PlanTest -> QueryTest -> SharedSparkSession chain.
Spark uses the AnyFunSuite ScalaTest style throughout, so the SparkTestSuite / PlanTestBase / QueryTestBase / SharedSparkSessionBase chain is not relevant to contributors picking a base for a new test suite. Collapse the section to just the class-bearing chain (SparkFunSuite -> PlanTest -> QueryTest -> SharedSparkSession), drop the SparkTestSuite row from the decision table, fold its helpers into the SparkFunSuite row, drop the now-redundant base-chain column from the session-provider table, and trim the linearization gotcha to no longer cite SparkTestSuite / AnyWordSpec. Generated-by: Claude opus-4-7
zhengruifeng
added a commit
to zhengruifeng/spark
that referenced
this pull request
May 7, 2026
…les_for_files ### What changes were proposed in this pull request? This PR extends `determine_modules_for_files` in `dev/sparktestsupport/utils.py` to ignore `AGENTS.md` and `CONTRIBUTING.md` in addition to the existing `README.md`. ### Why are the changes needed? A documentation-only PR that touches only `AGENTS.md` (e.g. apache#55707) currently triggers all CI test jobs because the file is not associated with any submodule, so it falls through to the `root` module. Neither file affects code or tests, and neither is consumed by the docs build, so they should be ignored just like `README.md`. ### Does this PR introduce _any_ user-facing change? No, this is only a testing infra change. ### How was this patch tested? Updated and ran the doctests in `dev/sparktestsupport/utils.py`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Add a
Scala Test Base Classessection toAGENTS.mdthat documents the layered Scala test base hierarchy in this repo and how to pick a base class for a new test suite. Spark uses theAnyFunSuiteScalaTest style throughout, and the chain is:The new section also includes:
QueryTest, full SQL with session) to the right base.QueryTestdeclaresspark: SparkSessionabstractly viaSparkSessionProvider, plus a session-provider table listing the two formalSparkSessionProviderimplementations in the repo:SharedSparkSession(sql/core) andTestHiveSingleton(sql/hive).extendsclause must transitively extend a class. Pure helper traits (*ErrorsBase,*Helper) cannot be put first.CLAUDE.mdis a symlink toAGENTS.md, so this change is picked up by both AI agent toolchains.Why are the changes needed?
Picking the wrong test base class (e.g. extending
QueryTestdirectly when a session is needed, orSparkFunSuitewhenPlanTestwould do) is a common stumble when adding new Scala test suites. The information is currently spread across the source ofSparkFunSuite,PlanTest,QueryTest, andSharedSparkSession, with no single place that summarizes when to use which. Documenting it inAGENTS.mdgives both contributors and AI coding agents a quick reference.Does this PR introduce any user-facing change?
No. Documentation-only change to a developer/agent guide file.
How was this patch tested?
N/A. Documentation-only change; no code or tests are affected.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude opus-4-7